/run-all-tests

/run-all-tests

@july2993 PTAL

@july2993 All go functions have been deleted.

In what case is a buffer size as large as 65536 useful for the syncer? Maybe we can simply make it much shorter by default, for example, 50 or 100. So that even when the downstream database is severely out of sync with the upstream, it's less likely to cause OOM.

I think OOM is unavoidable, for example, the machine running drainer may have only 4 GB of memory, but the average size of a sequence of binlogs is more than 0.4 GB, then even with this new configuration, there wouldn't be enough memory because there's a buffered channel with size 10 for each pump instance.

• In what case is a buffer size as large as 65536 useful for the syncer? Maybe we can simply make it much shorter by default, for example, 50 or 100. So that even when the downstream database is severely out of sync with the upstream, it's less likely to cause OOM.

  • I agree, but the default value should be best larger than worker-count*batch-size

• I think OOM is unavoidable, for example, the machine running drainer may have only 4 GB of memory, but the average size of a sequence of binlogs is more than 0.4 GB, then even with this new configuration, there wouldn't be enough memory because there's a buffered channel with size 10 for each pump instance.

  • we can change default 4G to be some portion of the total memory of the machine.
  • another problem is how to control the limit cache memory binlog as accurate as possible.

• In what case is a buffer size as large as 65536 useful for the syncer? Maybe we can simply make it much shorter by default, for example, 50 or 100. So that even when the downstream database is severely out of sync with the upstream, it's less likely to cause OOM.
  • How about setting the default buffer size to worker-count*batch-size, but user can still reset this variable in config?
• I think OOM is unavoidable, for example, the machine running drainer may have only 4 GB of memory, but the average size of a sequence of binlogs is more than 0.4 GB, then even with this new configuration, there wouldn't be enough memory because there's a buffered channel with size 10 for each pump instance.
  • It seems not easy for golang to count how much memory has a variable occupied. Current statistics is smaller than the real.

@lichunzhu you can fire another pr to change the default buffer size first.

Note that the unit of worker-count * batch-size is DML or DDL, which means 1 binlog in the buffer may correspond to multiple units? So there doesn't exist a one to one relationship between the two?

@suzaku IMO, worker-count * batch-size is the maximun cached DMLs' size in loader. If loader received a DDL the cached DMLs will be all executed and the cache will be cleared. So I think it's OK to use worker-count * batch-size.

Current cfg.MaxCacheBinlogSize is an int64 variable which is not easy for customers to modify. If the other parts are OK I will change this to HumanizeBytes which is used in pump/storage.go:StopWriteAtAvailableSpace. The customer can use 4 GB to represent the memory usage limit they want.

@suzaku PTAL

I think the problem to introduce this new configuration is that it can't effectively solve the OOM problem.

The "cache" is one of the largest buffer with the default configuration, but it's by no means the only data structure that may eat up a lot of memory.

It's "obviously" the bottleneck because it's 65536 by default, with a big worker-count and batch-size the bottleneck would be elsewhere.

This limitation add complexity to both our code and the user interface and can't actually solve the problem, so I suggest we don't add this limitation that only apply the one single data structure.

I think the problem to introduce this new configuration is that it can't effectively solve the OOM problem.

The "cache" is one of the largest buffer with the default configuration, but it's by no means the only data structure that may eat up a lot of memory.

It's "obviously" the bottleneck because it's 65536 by default, with a big worker-count and batch-size the bottleneck would be elsewhere.

This limitation add complexity to both our code and the user interface and can't actually solve the problem, so I suggest we don't add this limitation that only apply the one single data structure.

@suzaku If a client's binlogs are all extremely huge, I think this kind of limitations can take effects. The number of cached binlogs can't explicitly reprent the memory usage of syncer.

But if the binlogs are all extremely large, then other data structures would still eat up the memory and cause OOM.